fix: expand actively_monitored_wells to include wells from all groups(BDMS-974/1178) - #866
Open
likithabommasani21 wants to merge 6 commits into
Open
Conversation
Contributor
Coverage✅ 79.19% total — gate is 75%. No measured coverage for the Python files changed here. |
likithabommasani21
force-pushed
the
BDMS-974-a-4-fix-brittle-sql-filter-in-actively-monitored-wells
branch
from
August 20, 2026 18:52
4d15dd1 to
1112edd
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Expands actively monitored well OGC views to include wells from all groups while retaining the current monitoring filter.
Changes:
- Recreates public and internal views without the group-name restriction.
- Adds coverage for wells in other groups.
- Updates related OGC cleanup scenarios.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Summary |
|---|---|
tests/test_ogc.py |
Adds regression coverage for wells in non-default groups. |
tests/features/ogc-cleanup-sprint1.feature |
Updates A4/A6 OGC scenarios and removes stale assertions. |
alembic/versions/986e0eb85ab3_expand_actively_monitored_wells_to_all_.py |
Requires a public group-release filter (critical, 3 votes), coverage for the internal mirror (moderate, 2 votes), and unique feature IDs for per-group rows (critical, 1 vote). |
Suppressed comments (1)
tests/test_ogc.py:597
- This migration changes both the public and internal views, but the new regression test only queries
ogc_actively_monitored_wells. A regression in_create_internal_actively_monitored_wells_view()would still pass, leaving/ogcapi-internalwith the old group-name restriction; run the same assertion against both relations (or exercise the internal endpoint) so both changed definitions are covered.
row = session.execute(
text(
"SELECT group_id, group_name, group_type "
"FROM ogc_actively_monitored_wells WHERE id = :thing_id"
),
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
jirhiker
force-pushed
the
BDMS-974-a-4-fix-brittle-sql-filter-in-actively-monitored-wells
branch
from
August 22, 2026 22:55
05a6436 to
cc19645
Compare
This was referenced Aug 23, 2026
…d_wells view and added tests for internal"
…keep id unique and updated tests accordingly.
likithabommasani21
force-pushed
the
BDMS-974-a-4-fix-brittle-sql-filter-in-actively-monitored-wells
branch
from
August 24, 2026 17:38
9cc8ae7 to
1bdabcc
Compare
…th the renamed group_ids/group_names/group_types columns
…ligned by group_id
jirhiker
approved these changes
Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BDMS-974/1178: Expand actively_monitored_wells to include wells from all groups
Why
How
New Alembic migration (986e0eb85ab3) drops and recreates both ogc_actively_monitored_wells and its internal mirror ogc_internal_actively_monitored_wells, removing the group-name predicate. The status_value = 'Currently monitored' filter is untouched — that's the real definition of "actively monitored."
Verified against production data first: confirmed via a real query that no currently-monitored well has zero group memberships, so the existing inner-join structure is safe as-is (no need to switch to a LEFT JOIN).
Wells belonging to multiple groups are aggregated into one row (group_ids/group_names/group_types as parallel arrays) rather than one row per group, so id stays unique for pygeoapi's id_field: id lookups — verified live that duplicate rows silently broke /items/{id}.
Deduplicates group_thing_association rows first (no unique constraint exists on that table) and orders all three arrays by group_id, so they stay correctly aligned with each other rather than each being sorted independently.
Added a test (test_ogc_actively_monitored_wells_includes_wells_from_other_groups) proving a well in a group other than Water Level Network now shows up.
Rewrote the A4/A6 scenarios in ogc-cleanup-sprint1.feature, dropping the old rename/deprecation-header scenarios and the stale hardcoded "322" feature-count assertion (never matched real data in any environment we checked).
Notes